Feature: synchronizing and networking modules #30
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: Dmitriy Khaustov aka xDimon <[email protected]>
Signed-off-by: Dmitriy Khaustov aka xDimon <[email protected]>
There was a problem hiding this comment.
Pull Request Overview
Initial setup for synchronizing and networking modules
- Add core and stub types in
jam_types, including request/response attributes - Integrate and register
NetworkingLoaderandSynchronizerLoaderin the injector - Extend CLI/configuration with
base_pathandmodules_dirsupport and validation
Reviewed Changes
Copilot reviewed 68 out of 68 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/jam_types/types.tmp.hpp | Define stub types, block types, enums, bit-masks, and formatters |
| src/injector/node_injector.hpp/.cpp | Remove old forward decl, bind new loaders, refactor dispatcher scope |
| src/executable/jam_node.cpp | Switch to dynamic modulesDir(), handle unsupported loaders, remove unused includes |
| src/crypto/*.hpp | Replace qtils::BytesN with qtils::ByteArr in all crypto type aliases |
| src/app/impl/state_manager_impl.* | Migrate from shared_ptr to qtils::SharedRef for logging |
| src/app/impl/application_impl.* | Migrate to qtils::SharedRef, simplify metrics setup via GaugeHelper |
| src/app/configurator.hpp/.cpp | Add base_path/modules_dir CLI options, parse/validate YAML, error reporting |
| src/app/configuration.hpp/.cpp | Add getters for basePath() and modulesDir() |
| src/app/CMakeLists.txt | Link qtils::qtils to state manager |
| src/CMakeLists.txt | Broaden include directories to source and generated paths |
| scripts/asn1.py | Update codegen templates to use ByteArr/ByteVec instead of BytesN/Bytes |
| example/config.yaml | Add base_path and modules_dir examples; tweak logging levels and module groups |
Comments suppressed due to low confidence (4)
src/jam_types/types.tmp.hpp:51
- The enum member name
_MASKuses a leading underscore with capital letters, which is reserved by C++. Consider renaming it tokMaskorMaskto follow identifier conventions and avoid UB.
_MASK = 0b11111,
src/app/configurator.hpp:89
- [nitpick] CLI option names with underscores (e.g.
--base_path) are unconventional; consider using hyphenated flags (e.g.--base-path) to match POSIX conventions.
("base_path", po::value<std::string>(), "Set base path. All relative paths will be resolved based on this path.")
src/app/configuration.hpp:27
- [nitpick] Public API method
basePath()is undocumented. Please add a brief doc comment explaining what path this returns and its intended use.
[[nodiscard]] std::filesystem::path basePath() const;
src/injector/node_injector.cpp:126
- [nitpick] The new branches for
NetworkingLoaderandSynchronizerLoaderin the loader registration logic aren't covered by unit tests. Consider adding tests to validate correct loader selection and error handling.
else if ("NetworkingLoader" == module->get_loader_id()) {
xDimon
force-pushed
the
feature/basic_modules
branch
from
a9a7074 to
b65e1c2
Compare
turuslan
approved these changes
Jun 16, 2025
Signed-off-by: Dmitriy Khaustov aka xDimon <[email protected]> # Conflicts: # src/executable/jam_node.cpp
Signed-off-by: Dmitriy Khaustov aka xDimon <[email protected]> # The commit message #2 will be skipped: # + base # # Signed-off-by: Dmitriy Khaustov aka xDimon <[email protected]>
Signed-off-by: Dmitriy Khaustov aka xDimon <[email protected]>
Signed-off-by: Dmitriy Khaustov aka xDimon <[email protected]> # Conflicts: # src/modules/CMakeLists.txt # src/se/subscription_fwd.hpp
Signed-off-by: Dmitriy Khaustov aka xDimon <[email protected]>
xDimon
force-pushed
the
feature/basic_modules
branch
from
b65e1c2 to
047349a
Compare
Signed-off-by: Dmitriy Khaustov aka xDimon <[email protected]>
xDimon
force-pushed
the
feature/basic_modules
branch
from
c1e3862 to
cb44aa1
Compare
iceseer
approved these changes
Jun 17, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Init state of synchronizing and networking modules